Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-115999: Introduce helpers for (un)specializing instructions #126414

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Nov 4, 2024

Consolidate the code to specialize/unspecialize instructions into two helper functions and use them in _Py_Specialize_BinaryOp. The result is more concise and keeps all of the logic at the point where we decide to specialize/unspecialize an instruction.

If folks are amenable to this change, we can incrementally introduce their use as we work through making each specialization family thread-safe.

Consolidate the code to specialize/unspecialize instructions into
two helper functions and use them in `_Py_Specialize_BinaryOp`.
The resulting code is more concise and keeps all of the logic at
the point where we decide to specialize/unspecialize an instruction.

If folks are amenable to this change, we can incrementally introduce
their use as we work through making each specialization family
thread-safe.
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@mpage mpage merged commit 9ce4fa0 into python:main Nov 6, 2024
45 checks passed
@mpage mpage deleted the gh-115999-refactor-spec-helpers branch November 6, 2024 20:04
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Nov 6, 2024
@freakboy3742
Copy link
Contributor

Not sure what is going on here, but this has caused a mass failure of buildbots... I'm seeing it on the iOS buildbot, where it's failing to build the macOS host build, before it even gets to the iOS build:

gcc -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Python/specialize.o ../../Python/specialize.c
../../Python/specialize.c:2801:5: error: call to undeclared function 'SET_OPCODE_OR_RETURN'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    SET_OPCODE_OR_RETURN(instr, CONTAINS_OP);
    ^
1 error generated.
make: *** [Python/specialize.o] Error 1
program finished with exit code 2
elapsedTime=28.356593

@colesbury
Copy link
Contributor

@freakboy3742 - it's fixed in main now (by @Eclips4)

@mpage
Copy link
Contributor Author

mpage commented Nov 6, 2024

Not sure what is going on here, but this has caused a mass failure of buildbots... I'm seeing it on the iOS buildbot, where it's failing to build the macOS host build, before it even gets to the iOS build:

gcc -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Python/specialize.o ../../Python/specialize.c
../../Python/specialize.c:2801:5: error: call to undeclared function 'SET_OPCODE_OR_RETURN'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    SET_OPCODE_OR_RETURN(instr, CONTAINS_OP);
    ^
1 error generated.
make: *** [Python/specialize.o] Error 1
program finished with exit code 2
elapsedTime=28.356593

Sorry this was my fault. I merged this after #126450 was merged, without merging main into this PR first. This should be fixed by #126517.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants